Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New benchmarks in java #222

Open
wants to merge 5 commits into
base: java_benchmarks_support
Choose a base branch
from

Conversation

mahlashrifi
Copy link
Collaborator

@mahlashrifi mahlashrifi commented Oct 17, 2024

Hi,
I hope you are doing well. I have made a pull request to clarify which parts are unnecessary and which parts need to be added.
This pull request contains three components:

1- Completion of Java support for benchmarking on OpenWhisk (when the fat jar, including wrapper and source code, is less than 48 MB).
The initial commits are included in this pull request.

2- Two new benchmarks:
Training function of TeaStore
CheckLogin benchmark of TeaStore

3- Six additional benchmarks that are essentially variations of CheckLogin with different dependencies. I noticed something called "payload" in the results of running SeBS benchmarks, but I wasn’t sure how to use it. These six benchmarks were meant to serve as examples for using our implementation. My intention was to test whether the jar file size directly affects the delay in running serverless functions, but I found no significant impact.
The attached diagram and summarized information provide further details. I believe that if we can set the payload in SeBS, these additional codes can be omitted.
summary2
size_vs_time

@mahlashrifi mahlashrifi changed the title Java benchmarks support New benchmarks in java Oct 17, 2024
@@ -0,0 +1,33 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.vscode should be deleted :-)

@@ -0,0 +1,10 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should be deleted

@@ -168,7 +168,7 @@ def build_base_image(
)

for fn in os.listdir(directory):
if fn not in ("index.js", "__main__.py", "Main.java"):
if fn not in ("index.js", "__main__.py"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the .java fil deleted now?

stdout=subprocess.PIPE,
check=True,
)
if code_package.language_name == 'java':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why we need to specify this argument for Java, and not for Python and Nodejs?

)

else:
subprocess.run(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slightly less verbose version: just define the array of arguments for run, and only if language is Java, then append the two arguments of main.

@@ -1,6 +1,6 @@
{
"general": {
"docker_repository": "spcleth/serverless-benchmarks"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine of course for testing but it shouldn't be committed :-)

@@ -0,0 +1,38 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also shouldn't be commited

@@ -5,67 +5,11 @@
"update_storage": false,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should not be committed.

jsonObject.put("compute_time", computeTime);
jsonObject.put("is_cold", isCold);
jsonObject.put("result", result);
jsonResult.addProperty("begin", formattedBegin);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious - what's the difference here :-)?

@@ -0,0 +1,6 @@
{
"timeout": 60,
"memory": 24,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't appear to be a correct value - is it really just 24 MB?

@mcopik
Copy link
Collaborator

mcopik commented Dec 20, 2024

@mahlashrifi Few comments in addition to the review :-)

  • I'm a bit confused about the directory structure: why some many folders look the same and are named benchmark${i}?
  • We should annotate the code from TrainTicket benchmark with their original license - we should add in the beginning of the file the contents of their Apache license + copyright of original authors.
  • We should create a separate directory for the whole microservice - but let's discuss it once we resolve other issues :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants